Skip to content

chore: remove unused S3 SignatureVersion setting#39244

Open
ricardogarim wants to merge 2 commits intochore/aws-sdk-v3from
chore/remove-s3-signatureversion-setting
Open

chore: remove unused S3 SignatureVersion setting#39244
ricardogarim wants to merge 2 commits intochore/aws-sdk-v3from
chore/remove-s3-signatureversion-setting

Conversation

@ricardogarim
Copy link
Contributor

@ricardogarim ricardogarim commented Mar 2, 2026

Proposed changes (including videos or screenshots)

  • Remove the remaining (now unused) SignatureVersion-related code from the S3 config.
  • Drop the corresponding i18n entries for this setting across all locales.

The AWS SDK v3 S3 client always uses Signature Version 4 and no longer accepts a configurable signature version, so this setting had become a no-op.

Issue(s)

Steps to test or reproduce

Further comments

Summary by CodeRabbit

  • Refactor
    • Removed the S3 Signature Version configuration option from file upload settings.
  • Localization
    • Removed the "Signature Version" translation key across multiple locales, so that label will no longer appear in localized UIs.

@dionisio-bot
Copy link
Contributor

dionisio-bot bot commented Mar 2, 2026

Looks like this PR is not ready to merge, because of the following issues:

  • This PR is missing the 'stat: QA assured' label

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 2, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c84b6d3 and 36f4192.

📒 Files selected for processing (55)
  • packages/i18n/src/locales/af.i18n.json
  • packages/i18n/src/locales/ar.i18n.json
  • packages/i18n/src/locales/az.i18n.json
  • packages/i18n/src/locales/be-BY.i18n.json
  • packages/i18n/src/locales/bg.i18n.json
  • packages/i18n/src/locales/bs.i18n.json
  • packages/i18n/src/locales/ca.i18n.json
  • packages/i18n/src/locales/cs.i18n.json
  • packages/i18n/src/locales/cy.i18n.json
  • packages/i18n/src/locales/da.i18n.json
  • packages/i18n/src/locales/de-AT.i18n.json
  • packages/i18n/src/locales/de-IN.i18n.json
  • packages/i18n/src/locales/de.i18n.json
  • packages/i18n/src/locales/el.i18n.json
  • packages/i18n/src/locales/eo.i18n.json
  • packages/i18n/src/locales/es.i18n.json
  • packages/i18n/src/locales/fa.i18n.json
  • packages/i18n/src/locales/fi.i18n.json
  • packages/i18n/src/locales/fr.i18n.json
  • packages/i18n/src/locales/hi-IN.i18n.json
  • packages/i18n/src/locales/hr.i18n.json
  • packages/i18n/src/locales/hu.i18n.json
  • packages/i18n/src/locales/id.i18n.json
  • packages/i18n/src/locales/it.i18n.json
  • packages/i18n/src/locales/ja.i18n.json
  • packages/i18n/src/locales/ka-GE.i18n.json
  • packages/i18n/src/locales/km.i18n.json
  • packages/i18n/src/locales/ko.i18n.json
  • packages/i18n/src/locales/ku.i18n.json
  • packages/i18n/src/locales/lo.i18n.json
  • packages/i18n/src/locales/lt.i18n.json
  • packages/i18n/src/locales/lv.i18n.json
  • packages/i18n/src/locales/mn.i18n.json
  • packages/i18n/src/locales/ms-MY.i18n.json
  • packages/i18n/src/locales/nb.i18n.json
  • packages/i18n/src/locales/nl.i18n.json
  • packages/i18n/src/locales/nn.i18n.json
  • packages/i18n/src/locales/pl.i18n.json
  • packages/i18n/src/locales/pt-BR.i18n.json
  • packages/i18n/src/locales/pt.i18n.json
  • packages/i18n/src/locales/ro.i18n.json
  • packages/i18n/src/locales/ru.i18n.json
  • packages/i18n/src/locales/sk-SK.i18n.json
  • packages/i18n/src/locales/sl-SI.i18n.json
  • packages/i18n/src/locales/sq.i18n.json
  • packages/i18n/src/locales/sr.i18n.json
  • packages/i18n/src/locales/sv.i18n.json
  • packages/i18n/src/locales/ta-IN.i18n.json
  • packages/i18n/src/locales/th-TH.i18n.json
  • packages/i18n/src/locales/tr.i18n.json
  • packages/i18n/src/locales/uk.i18n.json
  • packages/i18n/src/locales/vi-VN.i18n.json
  • packages/i18n/src/locales/zh-HK.i18n.json
  • packages/i18n/src/locales/zh-TW.i18n.json
  • packages/i18n/src/locales/zh.i18n.json
💤 Files with no reviewable changes (46)
  • packages/i18n/src/locales/ru.i18n.json
  • packages/i18n/src/locales/ja.i18n.json
  • packages/i18n/src/locales/de-AT.i18n.json
  • packages/i18n/src/locales/lv.i18n.json
  • packages/i18n/src/locales/uk.i18n.json
  • packages/i18n/src/locales/pt.i18n.json
  • packages/i18n/src/locales/ko.i18n.json
  • packages/i18n/src/locales/es.i18n.json
  • packages/i18n/src/locales/km.i18n.json
  • packages/i18n/src/locales/de-IN.i18n.json
  • packages/i18n/src/locales/sr.i18n.json
  • packages/i18n/src/locales/af.i18n.json
  • packages/i18n/src/locales/pt-BR.i18n.json
  • packages/i18n/src/locales/vi-VN.i18n.json
  • packages/i18n/src/locales/nl.i18n.json
  • packages/i18n/src/locales/mn.i18n.json
  • packages/i18n/src/locales/ar.i18n.json
  • packages/i18n/src/locales/ms-MY.i18n.json
  • packages/i18n/src/locales/zh-HK.i18n.json
  • packages/i18n/src/locales/ro.i18n.json
  • packages/i18n/src/locales/sl-SI.i18n.json
  • packages/i18n/src/locales/sq.i18n.json
  • packages/i18n/src/locales/el.i18n.json
  • packages/i18n/src/locales/it.i18n.json
  • packages/i18n/src/locales/pl.i18n.json
  • packages/i18n/src/locales/lo.i18n.json
  • packages/i18n/src/locales/zh-TW.i18n.json
  • packages/i18n/src/locales/ka-GE.i18n.json
  • packages/i18n/src/locales/eo.i18n.json
  • packages/i18n/src/locales/fr.i18n.json
  • packages/i18n/src/locales/sv.i18n.json
  • packages/i18n/src/locales/id.i18n.json
  • packages/i18n/src/locales/lt.i18n.json
  • packages/i18n/src/locales/nb.i18n.json
  • packages/i18n/src/locales/th-TH.i18n.json
  • packages/i18n/src/locales/ku.i18n.json
  • packages/i18n/src/locales/fa.i18n.json
  • packages/i18n/src/locales/zh.i18n.json
  • packages/i18n/src/locales/hu.i18n.json
  • packages/i18n/src/locales/sk-SK.i18n.json
  • packages/i18n/src/locales/hi-IN.i18n.json
  • packages/i18n/src/locales/hr.i18n.json
  • packages/i18n/src/locales/fi.i18n.json
  • packages/i18n/src/locales/tr.i18n.json
  • packages/i18n/src/locales/ta-IN.i18n.json
  • packages/i18n/src/locales/nn.i18n.json
✅ Files skipped from review due to trivial changes (2)
  • packages/i18n/src/locales/cs.i18n.json
  • packages/i18n/src/locales/bs.i18n.json
📜 Recent review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: 📦 Build Packages
  • GitHub Check: cubic · AI code reviewer
  • GitHub Check: CodeQL-Build
  • GitHub Check: CodeQL-Build
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 0
File: :0-0
Timestamp: 2026-02-24T19:05:56.710Z
Learning: In Rocket.Chat PRs, keep feature PRs free of unrelated lockfile-only dependency bumps; prefer reverting lockfile drift or isolating such bumps into a separate "chore" commit/PR, and always use yarn install --immutable with the Yarn version pinned in package.json via Corepack.
📚 Learning: 2025-11-19T12:32:29.696Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37547
File: packages/i18n/src/locales/en.i18n.json:634-634
Timestamp: 2025-11-19T12:32:29.696Z
Learning: Repo: RocketChat/Rocket.Chat
Context: i18n workflow
Learning: In this repository, new translation keys should be added to packages/i18n/src/locales/en.i18n.json only; other locale files are populated via the external translation pipeline and/or fall back to English. Do not request adding the same key to all locale files in future reviews.

Applied to files:

  • packages/i18n/src/locales/be-BY.i18n.json
  • packages/i18n/src/locales/bg.i18n.json
🔇 Additional comments (7)
packages/i18n/src/locales/cy.i18n.json (1)

2594-2594: Looks good on this locale change.

Line 2594 keeps a valid key/value pair and does not introduce JSON or i18n-lookup issues.

packages/i18n/src/locales/ca.i18n.json (1)

4422-4422: Looks good — valid and stable i18n entry.

This localized string edit is syntactically correct JSON and does not introduce behavioral risk.

packages/i18n/src/locales/be-BY.i18n.json (1)

2626-2626: Looks good — formatting-only change with preserved translation intent.

Line 2626 keeps the same key and meaning, and the JSON remains valid.

packages/i18n/src/locales/az.i18n.json (1)

2604-2604: The FileUpload_S3_SignatureVersion removal looks good; minor unrelated formatting change noted.

The removal of the FileUpload_S3_SignatureVersion translation key (not visible in the diff since it's a deletion) correctly aligns with the PR objective—AWS SDK v3 S3 client always uses Signature Version 4, making this setting obsolete.

This line appears to have an unrelated formatting adjustment. While harmless, consider keeping commits focused on their stated purpose for cleaner git history.

packages/i18n/src/locales/bg.i18n.json (1)

2595-2595: Looks good to me.

This localization string update is safe and does not introduce functional risk.

packages/i18n/src/locales/da.i18n.json (1)

3868-3868: Looks good — translation key/value remains valid and clear.

No issue detected in this localized string update.

packages/i18n/src/locales/de.i18n.json (1)

5109-5109: Looks good; translation is clear and consistent.

This message reads naturally in German and matches the surrounding registration validation copy.


Walkthrough

Removed the FileUpload_S3_SignatureVersion setting and its usage from the Amazon S3 connection config and settings definition; removed the corresponding translation key from all locale files. Changes align with removing the S3 signatureVersion option (related to the AWS SDK migration).

Changes

Cohort / File(s) Summary
S3 config & settings
apps/meteor/app/file-upload/server/config/AmazonS3.ts, apps/meteor/server/settings/file-upload.ts
Deleted retrieval/assignment of FileUpload_S3_SignatureVersion and removed signatureVersion from S3 connection config and from the exported settings definition.
i18n locale removals
packages/i18n/src/locales/en.i18n.json, packages/i18n/src/locales/*
Removed the FileUpload_S3_SignatureVersion translation key across language locale files (many single-line deletions). Minimal formatting/spacing adjustments in a few locales.
Manifest / package
manifest-file-name, package.json
Minor deletions reflected (lines removed); no functional API/signature changes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the primary change: removing the unused S3 SignatureVersion setting across configuration files and i18n locales.
Linked Issues check ✅ Passed The PR fulfills CORE-1287 objective by removing AWS SDK v2-specific SignatureVersion configuration that is incompatible with v3, where S3 always uses Signature Version 4.
Out of Scope Changes check ✅ Passed All changes are scoped to removing SignatureVersion: deletion from S3 config, settings definition, and all i18n locale files. No unrelated modifications present.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@changeset-bot
Copy link

changeset-bot bot commented Mar 2, 2026

⚠️ No Changeset found

Latest commit: 36f4192

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@ricardogarim ricardogarim added this to the 8.3.0 milestone Mar 2, 2026
@codecov
Copy link

codecov bot commented Mar 2, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.72%. Comparing base (5930ce4) to head (36f4192).
⚠️ Report is 4 commits behind head on chore/aws-sdk-v3.

Additional details and impacted files

Impacted file tree graph

@@                Coverage Diff                @@
##           chore/aws-sdk-v3   #39244   +/-   ##
=================================================
  Coverage             70.72%   70.72%           
=================================================
  Files                  3195     3195           
  Lines                113106   113105    -1     
  Branches              20493    20512   +19     
=================================================
+ Hits                  79991    79997    +6     
- Misses                31065    31066    +1     
+ Partials               2050     2042    -8     
Flag Coverage Δ
unit 71.37% <ø> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ricardogarim ricardogarim marked this pull request as ready for review March 2, 2026 14:32
@ricardogarim ricardogarim requested a review from a team as a code owner March 2, 2026 14:32
Copy link
Member

@KevLehman KevLehman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kinda remember that if you remove the english translation, then all the other translations would be removed by lingohub at the next update 👀 (for the future)

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 58 files

@ricardogarim ricardogarim force-pushed the chore/remove-s3-signatureversion-setting branch from 55f2e1b to c84b6d3 Compare March 2, 2026 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants